Fix wrong unit in Flask new semconv test #2645

emdneto · 2024-06-28T03:43:42Z

Description

Changes duration to be measured in seconds during the test since in stable semconv, the unit changed from ms to s and we are recording in seconds

Probably Fixes #2596

instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py

xrmx

I don't think this is the only test that has the same duration calculation

xrmx

Given this is now in seconds, isn't 10 seconds a bit too much for delta?

emdneto · 2024-06-28T12:47:29Z

I don't think this is the only test that has the same duration calculation

Do you mean in other instrumentations? If so, I think yes, that's right it might have other tests. For Flask, I believe this is the only one that uses new_semconv. I think the other tests in Flask are correct because they are using the default opt-in (old semconv), and the histogram unit is in ms

xrmx · 2024-06-28T14:25:54Z

I don't think this is the only test that has the same duration calculation

Do you mean in other instrumentations? If so, I think yes, that's right. For Flask, I think this is the only one that uses new_semconv. I think the other ones are correct because they are using the default opt-in (old semconv), and the histogram unit is in ms

Missed the setUp doing the patching based on test names 😨 For the next conversion what about using a decorator so at least it's explicit that something is going on?

emdneto · 2024-06-28T14:43:48Z

Given this is now in seconds, isn't 10 seconds a bit too much for delta?

Makes sense

instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py

…_programmatic.py Co-authored-by: Riccardo Magliocchetti <[email protected]>

xrmx

Thanks!

change duration to seconds in new semconv test

2db3cfb

github-actions bot assigned ocelotl Jun 28, 2024

github-actions bot requested a review from ocelotl June 28, 2024 03:44

xrmx added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 28, 2024